Introduce Scope::NonGlobModule and Scope::GlobModule #144131

b-naber · 2025-07-18T12:52:06Z

Splits Scope::Module into Scope::NonGlobModule and Scope::GlobModule. This makes it easier to implement the open namespaces project, where we have to introduce a third scope. Introducing two separate scopes for a module was also a change requested by @petrochenkov, and should make it easier to output better diagnostics.

r? @petrochenkov

compiler/rustc_resolve/src/ident.rs

resolve: Remove `Scope::CrateRoot` Use `Scope::Module` with the crate root module inside instead, which should be identical. This is a simplification by itself, but it will be even larger simplification if something like rust-lang#144131 is implemented, because `Scope::CrateRoot` is also a module with two actual scopes in it (for globs and non-globs). I also did some renamings for consistency: - `ScopeSet::AbsolutePath` -> `ScopeSet::ModuleAndExternPrelude` - `ModuleOrUniformRoot::CrateRootAndExternPrelude` -> `ModuleOrUniformRoot::ModuleAndExternPrelude` - `is_absolute_path` -> `module_and_extern_prelude`

Rollup merge of #144368 - petrochenkov:rmrootscope, r=b-naber resolve: Remove `Scope::CrateRoot` Use `Scope::Module` with the crate root module inside instead, which should be identical. This is a simplification by itself, but it will be even larger simplification if something like #144131 is implemented, because `Scope::CrateRoot` is also a module with two actual scopes in it (for globs and non-globs). I also did some renamings for consistency: - `ScopeSet::AbsolutePath` -> `ScopeSet::ModuleAndExternPrelude` - `ModuleOrUniformRoot::CrateRootAndExternPrelude` -> `ModuleOrUniformRoot::ModuleAndExternPrelude` - `is_absolute_path` -> `module_and_extern_prelude`

resolve: Remove `Scope::CrateRoot` Use `Scope::Module` with the crate root module inside instead, which should be identical. This is a simplification by itself, but it will be even larger simplification if something like rust-lang/rust#144131 is implemented, because `Scope::CrateRoot` is also a module with two actual scopes in it (for globs and non-globs). I also did some renamings for consistency: - `ScopeSet::AbsolutePath` -> `ScopeSet::ModuleAndExternPrelude` - `ModuleOrUniformRoot::CrateRootAndExternPrelude` -> `ModuleOrUniformRoot::ModuleAndExternPrelude` - `is_absolute_path` -> `module_and_extern_prelude`

b-naber · 2025-08-01T09:21:53Z

@petrochenkov do you have time to review this in the next couple of days?

petrochenkov · 2025-08-01T09:45:57Z

Probably after #144579, this is low priority for me.

b-naber · 2025-08-01T09:54:56Z

I figured, I'll just re-roll then.

r? @rust-lang/compiler

petrochenkov · 2025-08-01T09:57:51Z

I still need to look, this is an important part of the resolve infrastructure.

b-naber · 2025-08-01T10:19:22Z

Other maintainers are also qualified to review this code. PR writers are allowed to re-roll reviewers after 2 weeks of inactivity; the fact that you just ignore this is kind of wild.

Kobzol · 2025-08-01T10:38:04Z

Vadim is the foremost expert on name resolution in the compiler, and if he wants to take a look at the PR, that is completely fine. No one is taking away your option of rerolling, but that doesn't mean that you get to choose the only reviewer. If there are other people with concerns or comments, their input also has to be taken into account, regardless of whether they are the currently assigned reviewer or not.

RalfJung · 2025-08-01T10:47:49Z

(Btw you just pinged the entire compiler team, more than 50 people. The way to re-roll is r? compiler, then it only pings the chosen reviewer. But even that is unlikely to work well since it doesn't take into account expertise, unlike the initial assignment.)

b-naber · 2025-08-01T11:19:24Z

Vadim is the foremost expert on name resolution in the compiler, and if he wants to take a look at the PR, that is completely fine. No one is taking away your option of rerolling, but that doesn't mean that you get to choose the only reviewer. If there are other people with concerns or comments, their input also has to be taken into account, regardless of whether they are the currently assigned reviewer or not.

I understand, but there must be some limit for how long someone can request to take a look at a PR; if the PR has low priority for the maintainer requesting a review, at some point there should still be a way to get this merged in a reasonable amount of time. Now obviously 2 weeks isn't anywhere close to such a limit, so I apologize for overreacting here.

petrochenkov · 2025-08-01T12:43:54Z

I'm certainly going to review this in some time.
I've been doing some experiments in #144368 and https://github.com/petrochenkov/rust/tree/extprel3 to understand how to do this better.
The second branch in particular, once finished, will give a simplified example of what this change should look like eventually.

petrochenkov · 2025-08-01T19:20:10Z

#144793 splits extern prelude into two scopes and demonstrates some things that need to be done for the module scope split as well.

Introduction of ScopeSet::ExternPrelude for doing searches just in the two extern prelude scopes, and its use in resolve_ident_in_module_unadjusted. ScopeSet::Module should also be introduced and behave similarly.
Replacement of the ad hoc MacroExpandedExternCrateCannotShadowExternArguments error with a regular restricted shadowing error (AmbiguityKind::MoreExpandedVsOuter), slightly specialized to produce better diagnostics.

petrochenkov · 2025-08-05T16:05:47Z

compiler/rustc_resolve/src/lib.rs

+                    non_glob_module_inserted = true;
+                }
+                Scope::GlobModule(module, _) => {
+                    if !non_glob_module_inserted {


non_glob_module_inserted will always be true as written.

petrochenkov · 2025-08-05T16:08:19Z

compiler/rustc_resolve/src/lib.rs

        self.visit_scopes(ScopeSet::All(TypeNS), parent_scope, ctxt, |this, scope, _, _| {
            match scope {
-                Scope::Module(module, _) => {
+                Scope::NonGlobModule(module, _) => {
                    this.traits_in_module(module, assoc_item, &mut found_traits);


This is an interesting case from the language point of view.

Typically shadowed traits do not stop being in scope.
E.g. this works

trait Tr { fn method(&self) {} } impl Tr for () {} fn main() { trait Tr {} // no method ().method(); // the first `Tr` is still in scope despite being shadowed }

But if a trait from a glob shadowed by something non-glob, then it is no longer a scope.

Seems like a bug, but not sure if we'll be able to fix it.

as a specific test:

mod glob { pub trait Tr { fn method(&self) {} } impl Tr for () {} } // a: use glob::*; // ok fn main() { // b: use glob::*; // error trait Tr {} // no method ().method(); }

petrochenkov · 2025-08-05T16:09:36Z

compiler/rustc_resolve/src/diagnostics.rs

@@ -1076,7 +1076,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                        }
                    }
                }
-                Scope::Module(module, _) => {
+                Scope::NonGlobModule(module, _) | Scope::GlobModule(module, _) => {
                    this.add_module_candidates(module, &mut suggestions, filter_fn, None);


No need to add candidates twice, Scope::GlobModule can be skipped like in #144793.

petrochenkov · 2025-08-05T16:10:23Z

compiler/rustc_resolve/src/diagnostics.rs

@@ -1487,9 +1487,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
            &parent_scope,
            ident.span.ctxt(),
            |this, scope, _use_prelude, _ctxt| {
-                let Scope::Module(m, _) = scope else {
-                    return None;
+                let m = match scope {


Suggested change

let m = match scope {

let Scope::Module(m, _) | Scope::GlobModule(module, _) = scope else {

b-naber · 2025-08-05T17:30:48Z

Thanks for the review, I'll address it tomorrow.

I have a question about ScopeSet::Module though. The idea is to just have a call to early_resolve_ident_in_lexical_scope for the ModuleOrUniformRoot::Module variant, right? We still need to return Weak in resolve_ident_in_module_unadjusted though, I believe, and the early_resolve_ident_in_lexical_scope call discards that information. How do you want to get the value for Weak here? Change the return type of early_resolve_ident_in_lexical_scope? Otherwise calculating the correct variant would require us to replicate a lot of the logic of resolve_ident_in_non_glob_module and resolve_ident_in_glob_module.

petrochenkov · 2025-08-05T20:03:46Z

Sorry, I had to go and didn't finish the review, I'll review the remaining part tomorrow.

The idea is to just have a call to early_resolve_ident_in_lexical_scope for the ModuleOrUniformRoot::Module variant, right?

Yeah, the main missing part is ScopeSet::Module and using it for resolving in ModuleOrUniformRoot::Module.
I didn't yet look in detail how Weak should be treated.
If resolve_ident_in_module_unadjusted is no longer called from early_resolve_ident_in_lexical_scope, then maybe it won't need to return Weak at all, I'm not sure.

petrochenkov · 2025-08-06T13:33:03Z

compiler/rustc_resolve/src/ident.rs

+    struct Flags: u8 {
+        const MACRO_RULES          = 1 << 0;
+        const NON_GLOB_MODULE      = 1 << 1;
+        const GLOB_MODULE          = 1 << 2;


The single MODULE flag could be kept, the new flags are used identically.

petrochenkov · 2025-08-06T13:34:43Z

compiler/rustc_resolve/src/ident.rs

-                    TypeNS => {
-                        ctxt.adjust(ExpnId::root());
-                        Scope::ExternPrelude
+                Scope::NonGlobModule(..) | Scope::GlobModule(..) if module_and_extern_prelude => {


Suggested change

Scope::NonGlobModule(..) | Scope::GlobModule(..) if module_and_extern_prelude => {

Scope::GlobModule(..) if module_and_extern_prelude => {

From non-glob module we go to glob module, not to extern prelude.

rustbot assigned petrochenkov Jul 18, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 18, 2025

This comment has been minimized.

Sign in to view

rust-cloud-vms bot force-pushed the scope-refactoring branch from f1097bc to c3185be Compare July 18, 2025 14:48

petrochenkov mentioned this pull request Jul 23, 2025

resolve: Remove Scope::CrateRoot #144368

Merged

petrochenkov reviewed Jul 23, 2025

View reviewed changes

compiler/rustc_resolve/src/ident.rs Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

introduce Scope::NonGlobModule and Scope::GlobModule

9c0ee44

rust-cloud-vms bot force-pushed the scope-refactoring branch from c3185be to 9c0ee44 Compare July 28, 2025 21:02

b-naber mentioned this pull request Jul 28, 2025

Implement Open API Namespace Support rust-lang/rust-project-goals#256

Open

3 tasks

rustbot assigned SparrowLii and unassigned petrochenkov Aug 1, 2025

petrochenkov self-assigned this Aug 1, 2025

petrochenkov reviewed Aug 5, 2025

View reviewed changes

lcnr mentioned this pull request Aug 6, 2025

shadowed traits from glob imports in the same scope get ignored #144993

Open

petrochenkov reviewed Aug 6, 2025

View reviewed changes

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 6, 2025

	let m = match scope {
	let Scope::Module(m, _) \| Scope::GlobModule(module, _) = scope else {

	Scope::NonGlobModule(..) \| Scope::GlobModule(..) if module_and_extern_prelude => {
	Scope::GlobModule(..) if module_and_extern_prelude => {

Introduce Scope::NonGlobModule and Scope::GlobModule #144131

Are you sure you want to change the base?

Introduce Scope::NonGlobModule and Scope::GlobModule #144131

Conversation

b-naber commented Jul 18, 2025

Uh oh!

This comment has been minimized.

Uh oh!

This comment was marked as resolved.

b-naber commented Aug 1, 2025

Uh oh!

petrochenkov commented Aug 1, 2025

Uh oh!

b-naber commented Aug 1, 2025

Uh oh!

petrochenkov commented Aug 1, 2025

Uh oh!

b-naber commented Aug 1, 2025

Uh oh!

Kobzol commented Aug 1, 2025

Uh oh!

RalfJung commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

b-naber commented Aug 1, 2025

Uh oh!

petrochenkov commented Aug 1, 2025

Uh oh!

petrochenkov commented Aug 1, 2025

Uh oh!

petrochenkov Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

petrochenkov Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

lcnr Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

petrochenkov Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

petrochenkov Aug 5, 2025

Choose a reason for hiding this comment

Uh oh!

b-naber commented Aug 5, 2025

Uh oh!

petrochenkov commented Aug 5, 2025

Uh oh!

petrochenkov Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

petrochenkov Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RalfJung commented Aug 1, 2025 •

edited

Loading